OPRUN-4691: Promote OLMLifecycleAndCompatibility feature gate to Default - #2920
Conversation
|
Pipeline controller notification For optional jobs, comment This repository is configured in: LGTM mode |
|
Hello @perdasilva! Some important instructions when contributing to openshift/api: |
📝 WalkthroughWalkthroughThis change enables Suggested reviewers: 🚥 Pre-merge checks | ✅ 15✅ Passed checks (15 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
/hold |
|
@perdasilva: This pull request references OPRUN-4691 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "5.0.0" version, but no target version was set. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
Enable the OLMLifecycleAndCompatibility feature gate in the Default feature set for SelfManaged clusters. Previously this gate was only enabled in TechPreviewNoUpgrade and DevPreviewNoUpgrade. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Per G. da Silva <pegoncal@redhat.com>
6fc0456 to
a1e80c1
Compare
|
/lgtm Tests are passing all except the 5 test requirement, which is explained in the PR description |
|
@JoelSpeed: Overrode contexts on behalf of JoelSpeed: ci/prow/verify-feature-promotion DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
Scheduling tests matching the |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: JoelSpeed The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/retest |
yuqi-zhang
left a comment
There was a problem hiding this comment.
lgtm as well - marking as approved based on Joel's comment
|
/unhold |
|
/verified by @perdasilva Console changes are present (Cluster compatibility and Support phase columns)
feature gate is enabled: |
|
@perdasilva: This PR has been marked as verified by DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
/retest |
|
/retest |
|
/retest |
|
/override ci/prow/verify-feature-promotion |
|
@JoelSpeed: Overrode contexts on behalf of JoelSpeed: ci/prow/verify-feature-promotion, ci/prow/verify-hypershift-integration DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
@perdasilva: all tests passed! Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
…udge (replaces #660) (#701) * test(ci): add adversarial payload false-revert eval cases Revives the three evidence-heavy payload-analysis eval cases from #660 on top of current main. These are the control-arm cases capturing payloads where the Payload Agent recommended incorrect reverts, and they exercise the skill's ability to distinguish well-supported revert candidates from false attributions. - case-018: mixed true/false attribution — keep openshift/oc#2279 while rejecting the cross-tenant etcd evidence attributed to openshift/hypershift#8871. - case-019: reject openshift/api#2920 and #2923; the apparent long operator waits come from disjoint interval arithmetic in the Origin monitor test. - case-020: reject openshift/ovn-kubernetes#3298 and openshift/cloud-provider-azure#164; reconstruct the ordered GCP/Azure infrastructure chains and distinguish triggers from amplifiers, detectors, and cleanup fallout. Cases are registered in the eval README case index. docs/index.html is regenerated via `make update` to clear pre-existing plugin-metadata drift so the strict plugin lint passes. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * chore(ci): bump ci plugin to 0.0.86 for new eval cases The check-version-bump CI gate requires a version bump for any change under plugins/ci/. Bump the plugin version and re-sync marketplace.json and docs/index.html via `make update`. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * test(ci): add point-in-time cutoff + integrity judge, trim case fields Address review feedback on the revived payload false-revert eval cases: - Drop the excessive adversarial annotation fields (required_claims, must_not_conclude, forbidden_revert_candidates, distractors, key_evidence, discriminating_signal) from cases 018/019/020. Nothing in the payload-analysis eval consumed them; the core false-positive check is already covered by expected_candidates: []. - Keep the point-in-time metadata (payload_completed_at in input, analysis_cutoff in annotations) and extend it to the existing cases 001-014, since this is the highest-value part — it lets the eval detect hindsight/cheating. - Pass the cutoff to the skill as `--as-of {payload_completed_at}` and instruct the agent to perform a strict point-in-time analysis. - Add the point_in_time_integrity LLM judge (min_mean 4.0) that scores evidence provenance against analysis_cutoff and flags post-hoc leakage (later reverts, subsequent payload outcomes, present-day PR state). Cases without a cutoff score max, so it is a no-op for any future non-point-in-time case. Deliberately left behind #660's programmatic trace-hygiene judge (heavy and brittle) and the case_constraints adversarial-field judge. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * feat(ci): support --as-of point-in-time cutoff in payload-analysis skill Add the --as-of TIMESTAMP flag to the payload-analysis argument contract and bound every external lookup and subagent investigation to the cutoff. Under --as-of the skill reasons only from evidence that existed when the payload completed: it ignores post-cutoff reverts, comments, and payload outcomes, caps the step-registry commit window at the cutoff, and never treats a later revert (or its absence) as causal evidence. This keeps the point-in-time eval from leaking post-cutoff signal into recommendations. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * test(ci): unify cutoff field name, add deterministic outcome judges Unify the point-in-time cutoff under one name: annotations.yaml now carries payload_completed_at, identical to input.yaml, instead of a differently-named analysis_cutoff. The duplication itself stays because the harness only exposes annotations.yaml to judges; the dataset schema now documents that constraint. Add deterministic check judges for the outcomes that were previously only graded by LLM rubric: expected_candidates_found (every expected candidate at/above min_confidence), no_unexpected_reverts (a hard false-revert gate — no non-RPM candidate at/above the revert threshold outside the expected set), failed_job_count_matches (tolerance 1), force_accept_matches, and expected_phase_matches. All gate at min_pass_rate 1.0, and the conditional ones use the harness if: field so inapplicable cases are skipped rather than auto-scored. Judge fixes: revert_scoring_accuracy had drifted from the skill (it described a retired 130-point rubric with a "single candidate" signal); it now attaches SKILL.md as context and judges against the current rubric, with a two-sided scale where a false revert scores 1 instead of falling through anchors written for the true-positive case. point_in_time_integrity uses if: instead of instructing the model to return max score for non-cutoff cases. Case fixes: case-008 records its real phase (Rejected in every historical run) instead of ""; case-010 notes now state the verified Insights API Gateway HTTP 500 story with all four job names; cases 010 and 012 declare expected_candidates: [] explicitly. Also enable parallelism: 3 and tag MLflow runs. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(ci): address CodeRabbit findings on --as-of and eval judges SKILL.md: Step 3.6 now defines a single until_timestamp (the earlier of the window end and the --as-of cutoff) passed whole to both step-registry queries, replacing the until=<until_date>T23:59:59Z templates that would mangle a mid-day cutoff or silently re-admit same-day post-cutoff commits. Step 6.3 requests createdAt and treats post-cutoff revert state as unavailable: a pre-cutoff revert that merged after the cutoff counts as still Open. Eval: expected_candidates_found now verifies expected_failing_jobs linkage (substring match, short names vs full periodic names); yaml_results_valid requires the canonical candidate type field and only demands pr_url for non-RPM candidates; the outputs schema documents type-specific candidate fields. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * refactor(ci): progressive disclosure + report template for payload-analysis Restructure the payload-analysis skill for progressive disclosure: - Drop the "Required Skills" preamble that force-loaded the payload-results-yaml and payload-autodl-json schema skills before any work began; each is now loaded via the Skill tool at its point of use (Steps 6.5 and 8), keeping early context lean. - Move the three run-once blobs out of SKILL.md into references/ loaded at the step that needs them: the Step 4 investigation-subagent prompt (investigation-subagent.md), the report content rules (report-guide.md), and the Step 9 completeness-review prompt (completeness-review.md). The scoring rubric stays in SKILL.md — it is per-run core and the eval's revert_scoring_accuracy judge attaches SKILL.md as rubric context. - Replace ~220 lines of inline HTML fragments and partial CSS with assets/report-template.html — a complete fill-in-the-blanks page (placeholders plus BEGIN/END conditional and repeatable blocks) that Step 7 copies and fills. The old prose said "follow the styling conventions of the existing report format", which every run reinterpreted; the template is now the single source of structure and styling, so reports come out consistent across runs. New design: phase hero, stat tiles, confidence meters with a Confidence column in the reverts table, status pills, and per-payload history cells (status always encoded as text or luminance alongside color). SKILL.md drops from 882 lines / 10.1k words to 581 lines / 7.6k words. Bump ci to 0.0.87; marketplace + docs synced via make update. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * refactor(ci): report template mirrors the production Payload Agent format Replace the experimental template design with the canonical production report format (structure and CSS taken verbatim from a real claude-payload-agent run), parameterized with placeholders and BEGIN/END conditional blocks: executive summary with per-job persistence, revert verdict with the Score column and itemized rationale, no-revert and force-accept variants, blocking-jobs summary with S/F history patterns, per-job collapsible details with candidates-table/candidates-none alternatives, RHCOS changes with RPM candidates and per-hop diffs, informing tests, and adversarial review. Production runs had already drifted between each other on CSS details; the template pins the format so every run renders identically. Sync references/report-guide.md and the Step 10 checklist to the same section list. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * test(ci): correct case-011 expected_failed_job_count to 2 The payload had two failed blocking jobs (aggregated-aws-ovn-upgrade-5.0-major and aggregated-gcp-ovn-upgrade-5.0-micro), both from the external Insights API Gateway HTTP 500 incident. Confirmed by two independent eval runs (Codex/Harbor and a local claude-opus-4-6 run); the old count of 1 was annotation error and cost both LLM judges points against a correct analysis. Notes now name the jobs. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(ci): verify revert-threshold evidence and add claim audit to review Three changes aimed at the recurring false-revert signature (four production incidents, all scoring 85-95 from correlation stacks that were never evidence-checked): - Step 6.1: the self-skepticism re-verification now runs for ANY score at or above the revert threshold (>= 85), not only on cap overflow. Every historical false revert scored 85-95 — below the old trigger. - Rubric: the error-message-match +20-30 tier now requires an observed artifact linking the failing operation to the modified code (stack frame, log line, event); shared subsystem vocabulary explicitly does not qualify and scores +10. - Step 9: the completeness reviewer gains a claim-audit pass for candidates >= 85. Scores change through exactly one mechanism: striking an itemized signal by citing the artifact evidence that contradicts it, then recomputing mechanically. Speculative objections remain inadmissible — preserving the guard against the earlier failure mode where valid reverts were downgraded on flimsy doubts. Eval config: declare score_range on the three numeric judges and move skill under execution per harness deprecation. Bump ci to 0.0.88. Gitignore /eval/ and /tmp/ local harness output. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * test(ci): add no_answer_access cheating detector to payload-analysis eval Deterministic judge that fails a case when the agent transcript touches annotations.yaml, traverses the eval dataset directory, or echoes annotation-only vocabulary (expected_candidates, force_accept_expected, ...). Verified retroactively clean against all 17 CI case transcripts from the PR head run and every local Luna/Opus run; the harness never stages annotations into the workspace, so any hit means the agent went looking. Scans the main transcript; separate subagent transcript files are out of reach. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * test(ci): narrow cheating detector to path patterns only Annotation field-name patterns could false-positive when the agent legitimately reasons about its output schema; the two path patterns (annotations.yaml, evals/cases/payload-analysis) are sufficient tells. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * test(ci): cheating detector scans merged events incl. subagent transcripts collect.py merges subagents/*.jsonl into per-case events.json, so scanning the serialized events covers subagent tool calls and text. Raw stdout remains the fallback when no events were captured. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * revert(ci): restore payload-analysis skill to pre-audit behavior Restores SKILL.md and completeness-review.md to the exact content the passing CI run (2089641369093017600, tested 572a6f3) executed. The unvalidated behavior changes from 8094b81 (>= 85 re-verification, error-match tier tightening, Step 9 claim audit) went out with no eval evidence, and their first measured run (2089697973024854016) regressed case-007: the claim audit stamped the historical wrong answer (cluster-version-operator#1309 @ 90) as verified while the true cause (operator-framework-olm#1256) went unscored. Measurement-side changes are kept: all judges including the no_answer_access cheating detector, annotation fixes, and .gitignore guards. Future skill behavior changes need control-vs-treatment eval evidence with repeats before merging. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(ci): correct payload-analysis ground truth for case-019 and case-020 case-019 was mislabeled as a false-revert trap with no expected candidate. The aws/azure 4.22->5.0 major-upgrade jobs fail because CVO #1427 (OTA-1997) added a Deployment-manifest template field the 4.22 CVO cannot render, so it silently skips its own Deployment during upgrade. This was quick-reverted by #1431 (TRT-2842) and re-landed as #1433, confirming #1427 as the real cause. Set has_revert_candidates=true and add #1427 (@100, aws/azure-ovn-upgrade) as the expected candidate; the api#2920/#2923 attributions remain red herrings. case-020 met all three Step 6.4 force-accept criteria — both blocking failures are temporary infrastructure (Azure 429 throttling / OSProvisioningTimedOut, GCP transient VIP reachability loss), no more than 2 blocking jobs, and 48.4h since the last accepted payload (>= 18h). Set force_accept_expected=true. Also normalize expected_failing_jobs across cases to semantic-core tokens for bidirectional substring matching, make the expected_candidates_found matcher match when either the annotation token or the reported job name contains the other, and bump the judge model to claude-opus-4-8. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * test(ci): raise payload-analysis LLM thresholds to 4.5 Tighten the quality bars now that the corrected ground truth produces consistently high scores: analysis_quality min_mean 3.5 -> 4.5 and revert_scoring_accuracy min_mean 3.0 -> 4.5. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * feat(ci): make required_skill_invocations judge harness-neutral Detect skill usage via either signal so the one payload-analysis eval config scores both Claude Code and Codex runs: - Signal A (Claude Code): a normalized `Skill` tool invocation, read from the merged event stream with a raw stdout JSONL fallback. - Signal B (Codex and any harness without a Skill tool): the skill's canonical SKILL.md H1 heading appears in the transcript, meaning the skill body was loaded and run. Codex has no `Skill` tool, so the prior stdout-JSONL parse always failed there. Runner selection stays CLI-overridable (--agent codex --effort xhigh --model ...) over the claude-code defaults; no second config. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * feat(ci): add no_model_refusal deterministic judge to payload-analysis Detect API-level model refusals (the model's safeguards flagged the request) in the agent or any subagent, and fail the case explicitly so the root cause is visible instead of surfacing downstream as a confusing "missing output files" failure. This is an environment/model signal, not a skill-quality one. Detection is structural to avoid false positives: - system record whose subtype starts with model_refusal (survives in both raw stdout stream-json and the flat merged events schema); - a synthetic assistant turn (model "<synthetic>") paired with stop_reason "refusal" on the same record — which excludes the <synthetic> 429 rate-limit record (stop_reason "stop_sequence"); - "safeguards flagged" text as corroboration only, never a standalone trigger. Reuses the no_answer_access corpus idiom (json.dumps(events) with a stdout fallback) and reports which corpus was scanned. Claude Code only for now: when the capture is not a Claude Code run (no system/init signature) the judge abstains (no score) rather than emit a misleading pass — Codex refusal capture shape is a TODO. Gated at min_pass_rate 1.0. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(ci): resolve payload-analysis dataset.path from config dir The harness resolves dataset.path against the config file's directory (EvalConfig.resolve_path uses config_dir = plugins/ci/evals), so the repo-root-relative value doubled to plugins/ci/evals/plugins/ci/evals/cases/payload-analysis and did not exist — workspace setup would error and score.py would load zero annotations. Use the config-dir-relative form (cases/payload-analysis), matching the majority of eval configs in the repo. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * chore: regenerate docs/index.html after upstream merge Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(ci): pass expected_candidates_found on explicit no-revert cases The judge gated on `if: annotations.get('expected_candidates')`, but an explicit empty list — the documented way to declare "no revert expected" — is falsy in Python, so the judge silently skipped (n/a) instead of passing on all five no-revert cases (009, 010, 011, 012, 020). Its check body already returns a pass for an empty expectation, so gate on presence (`is not None`) like failed_job_count_matches does. The false-positive direction remains enforced by no_unexpected_reverts. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * feat(ci): allow Codex network egress in payload-analysis eval The default workspace-write sandbox blocks DNS/network entirely, starving the snapshot-backed analysis of the live GCS/Prow, GitHub, and Sippy lookups it still needs. Enable network_access on the write sandbox via runner.settings, which only the Codex runner reads — the claude-code runner ignores it, so its behavior is unchanged. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * feat(ci): keep codex scratch off /tmp; score implicit no-revert cases Two payload-analysis eval changes: - Add a system-prompt line telling the agent (and, via relay, its subagents) to use $TMPDIR for temporary files and downloaded artifacts instead of /tmp, so codex runs don't fill the RAM-backed tmpfs. The top-level agent honors this; subagent propagation is best-effort since codex child processes don't inherit runner.system_prompt. - Broaden expected_candidates_found so a case declaring no-revert implicitly (has_revert_candidates: false with expected_candidates omitted) is scored vacuously, matching the explicit [] form. Skipped only when a case defines neither expectation. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> Co-authored-by: Stephen Benjamin <stbenjam+ai@redhat.com>

Summary
OLMLifecycleAndCompatibilityfeature gate in the Default feature set for SelfManaged clustersfeatures/features.go, generated feature gate manifests, andfeatures.md5 test rule exception request
We are requesting an exception to the 5-
It-block requirement for this test suite. The test covers five distinct scenarios but structures them asg.Bysteps within a singleItblock. The reason is architectural, not a coverage shortcut.Scenarios covered
x-acknowledge-experimentalheader → returns empty stream (experimental API contract enforced)Why a single
ItblockMeaningful functional coverage of this endpoint requires a catalog with known, controlled content. An existing cluster catalog cannot substitute: once this feature ships to Default, real catalogs will contain lifecycle data, making any query result non-deterministic and unassertable.
Building that controlled catalog image in-cluster via a
BuildConfigtakes approximately 22 seconds. Splitting into fiveItblocks would trigger a separate image build per test because:BeforeEachruns independently per node, so shared setup is not possibleBeforeAllis not an option: in Ginkgo's parallel runner it executes once per process, not once globally — with N parallel nodes we get N image buildsThe single
Itbuilds the image once, creates oneCatalogSource, opens one port-forward, and exercises all five scenarios against the same running pod. Coverage is equivalent to five separate tests.The only genuinely parallelizable addition would be an input-validation test sending syntactically invalid schema/package names against an existing catalog pod (the rejection occurs before catalog lookup and is deterministic regardless of catalog content). That path is already covered by unit tests in operator-registry, so we have not duplicated it here at the E2E layer.
Test plan
make verifypassesfeatures.md🤖 Generated with Claude Code